fix #6 #7 #8
fix #6 #7 #8
Conversation
sjanssen2
left a comment
sjanssen2
left a comment
There was a problem hiding this comment.
I generally like the idea and the cleanup of the existing test. I would love to see instructions on what the user can do with the information provided in the MANIFEST, i.e. how to obtain these files.
| - `summary.html`: a browser friendly file listing that will include all files at `[artifact-id]/[output-folder]` and | ||
| any `index.html` files in any subfolder. As a reminder, the Qiita nginx basic configuration allows to display/load any | ||
| html/JS available files; thus, able to display properly `index.html` files available | ||
| - `MANIFEST.txt`: a comprehensive list of all available files in the folder. |
There was a problem hiding this comment.
from the wording, it is not 100% clear to me if
- "will include all files at
[artifact-id]/[output-folder]" includes recursion through all sub-directories - "Qiita nginx basic configuration allows to display/load any html/JS available files" which lines in the example configuration refers to this? Can you point to those?
- which class of user can make use of the
MANIFEST.txt. Can he/she download the whole directory as a ZIP archive?
| The two main plugins using this output are: | ||
|
|
||
| - https://github.com/qiita-spots/qp-knight-lab-processing: which will generate an `[output-folder]` contaning all the logs, | ||
| files and summaries from BCL to clean FASTQ processing. Note that multiqc resoults are part of this and the outputs are |
There was a problem hiding this comment.
Can you add which user class can run this command? I figure it is only admins but no normal users, right?
| from unittest import main | ||
| from tempfile import mkdtemp | ||
| from os import remove | ||
| from os.path import exists, isdir, join, dirname, abspath | ||
| from inspect import currentframe, getfile | ||
| from shutil import rmtree, copytree | ||
| from json import dumps | ||
| from os import remove | ||
| from os.path import abspath, dirname, exists, isdir, join | ||
| from shutil import copytree, rmtree | ||
| from tempfile import mkdtemp | ||
| from unittest import main |
There was a problem hiding this comment.
hm, your editor seems to have flipped ordering of imports. Makes it harder to see actual differences :-/
| self._clean_up_files.extend([ff["filepath"]]) | ||
| for f in res["files"].values() | ||
| for ff in f |
There was a problem hiding this comment.
my intuition says that shorter variable names are inner elements. Here f is a list and ff a file-dict.
With three lines, we have more space, therefore I suggest to change f into files and ff into file
| print("-------------") | ||
| print("-------------") | ||
| print(html) | ||
| print("-------------") | ||
| print(EXP_HTML.format(aid=aid)) | ||
| print("-------------") | ||
| print("-------------") |
| separator = "|--" | ||
| for dpath, _, files in walk(folder): | ||
| # assuring same order, mainly for testing | ||
| files.sort() |
| index.append(("file", f"{dpath}/{f}")) | ||
| # if we are not at the top, we should only add | ||
| # the index.html files | ||
| elif "index.html" in files: |
There was a problem hiding this comment.
do we have to take care for different case spelling, e.g. a file with name Index.html?
| elif "index.html" in files: | ||
| index.append(("file", f"{dpath}/index.html")) | ||
|
|
||
| depth = dpath.replace(folder, "").count(sep) |
There was a problem hiding this comment.
is folder relative or absolute? What if the path contains the folder infix twice like job-output/4/job-output/4/subdir?
| with open(manifest_fp, "w") as of: | ||
| of.write("\n".join(manifest)) | ||
|
|
||
| links = [link % (manifest_fp[tlink:], "file", manifest_fp[tname:])] |
There was a problem hiding this comment.
would https://www.geeksforgeeks.org/python/python-os-path-relpath-method/ be a safer solution? Instead of performing string operations here?
qtp_job_output_folder/summary.py
Outdated
| for ft, f in index: | ||
| # to avoid any duplication of lines: | ||
| _link = link % (f[tlink:], ft, f[tname:]) | ||
| if _link not in links: | ||
| links.append(_link) |
sjanssen2
left a comment
sjanssen2
left a comment
There was a problem hiding this comment.
When looking closer at the currently failing test, I see your debug info:
-------------
<a href="./10/test_data/MANIFEST.txt" type="file" target="_blank">test_data/MANIFEST.txt</a><br/>
<a href="./10/test_data/file_1" type="file" target="_blank">test_data/file_1</a><br/>
<a href="./10/test_data/file_2" type="file" target="_blank">test_data/file_2</a><br/>
<a href="./10/test_data/folder_a/folder_b/index.html" type="file" target="_blank">test_data/folder_a/folder_b/index.html</a><br/>
<a href="./10/test_data/folder_1/index.html" type="file" target="_blank">test_data/folder_1/index.html</a><br/>
<a href="./10/test_data/test_data/folder_a/folder_b/index.html" type="file" target="_blank">test_data/test_data/folder_a/folder_b/index.html</a><br/>
<a href="./10/test_data/test_data/folder_1/index.html" type="file" target="_blank">test_data/test_data/folder_1/index.html</a>
-------------
<a href="./10/test_data/MANIFEST.txt" type="file" target="_blank">test_data/MANIFEST.txt</a><br/>
<a href="./10/test_data/file_1" type="file" target="_blank">test_data/file_1</a><br/>
<a href="./10/test_data/file_2" type="file" target="_blank">test_data/file_2</a><br/>
<a href="./10/test_data/folder_a/folder_b/index.html" type="file" target="_blank">test_data/folder_a/folder_b/index.html</a><br/>
<a href="./10/test_data/folder_1/index.html" type="file" target="_blank">test_data/folder_1/index.html</a>
-------------
and assume the above is the observation, below is expectation.
Why do you observe ./10/test_data/test_data/folder_1/index.html? The test_data part should not be repeated?! As the provided test_data does NOT contain itself.
2 participants
No description provided.